-
Notifications
You must be signed in to change notification settings - Fork 486
feat: Add create-namespace
flag to vcluster platform add cluster
command
#2672
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for vcluster-docs canceled.Built without sensitive environment variables
|
create-namespace
flag to vcluster pro add cluster
commandcreate-namespace
flag to vcluster platform add cluster
command
if os.Getenv("DEVELOPMENT") == "true" { | ||
helmArgs = []string{ | ||
"upgrade", "--install", "loft", cmp.Or(os.Getenv("DEVELOPMENT_CHART_DIR"), "./chart"), | ||
"--create-namespace", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this one needs to stay, because we are overwriting helmArgs here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'm done, why os.Getenv("DEVELOPMENT") == "true"
needs to be forced to pass --create-namespace
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't need to be forced, but how the code was before the helmArgs isn't used inside DEV conditional, meaning you could not have turned it on. You can make it optional in this block, but needs to use the flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, how about I add a --dev-create-namespace
flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you misunderstand. My point is that this part:
if cmd.CreateNamespace {
helmArgs = append(helmArgs, "--create-namespace")
}
Should take effect in both the development section above (around line 172) and the non-development section below (which it already does). The flag should work whether you are in dev or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I understand that in development mode the helmArgs
string slice is reinitialized.
What issue type does this pull request address? (keep at least one, remove the others)
/kind feature
What does this pull request do? Which issues does it resolve? (use
resolves #<issue_number>
if possible)resolves #2546